-
Notifications
You must be signed in to change notification settings - Fork 2.8k
ZEPPELIN-1319 Use absolute path for ssl truststore and keystore when available #1319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Good catch, LGTM! CI failure looks unrelated. |
| getConfDir(), | ||
| getString(ConfVars.ZEPPELIN_SSL_KEYSTORE_PATH))); | ||
| String path = getString(ConfVars.ZEPPELIN_SSL_KEYSTORE_PATH); | ||
| if (path != null && path.startsWith("/")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it need to consider Windows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@felixcheung check for windows path is handled in getRelativeDir
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, if path == C:\zeppelin\conf\ssh
isn't this turning it into a broken path like c:\zeppelin\conf/c:\zeppelin\conf\ssh?
getRelativeDir(
String.format("%s/%s",
getConfDir(),
getString(path)));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 5587d5a
|
@prabhjyotsingh Should we merge this? |
|
@corneadoug Yes, sure. Will merge this soon if no more discussion. |
…available ### What is this PR for? Use absolute path for ssl truststore and keystore when available ### What type of PR is it? Improvement ### Todos * [ ] - Task ### What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-1319 ### How should this be tested? Config `zeppelin.ssl.truststore.path`, `zeppelin.ssl.keystore.path` and verify whether the absolute path or the path relative to conf is used. ### Screenshots (if appropriate) ### Questions: * Does the licenses files need update? n/a * Is there breaking changes for older versions? n/a * Does this needs documentation? n/a Author: Renjith Kamath <[email protected]> Closes #1319 from r-kamath/ZEPPELIN-1319 and squashes the following commits: 5587d5a [Renjith Kamath] ZEPPELIN-1319 add check for Windows path fc2ac9f [Renjith Kamath] ZEPPELIN-1319 Use absolute path for ssl truststore and keystore when available (cherry picked from commit 922364f) Signed-off-by: Prabhjyot Singh <[email protected]>
What is this PR for?
Use absolute path for ssl truststore and keystore when available
What type of PR is it?
Improvement
Todos
What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1319
How should this be tested?
Config
zeppelin.ssl.truststore.path,zeppelin.ssl.keystore.pathand verify whether the absolute path or the path relative to conf is used.Screenshots (if appropriate)
Questions: